Add synchronous execution option to workflow provisioning #990

junweid62 · 2025-01-08T21:30:28Z

Jan 14 Revision
Added synchronous execution option to reprovision

Description

This PR introduces a new wait_for_completion_timeout feature to the Provision Workflow API in the OpenSearch Flow Framework. The feature allows users to control whether the API call waits for the entire workflow provisioning process to complete before returning a response.

What’s Changed:

Added support for the wait_for_completion_timeout parameter in the REST layer (RestProvisionWorkflowAction).

Accepts a time duration value (e.g., 30s, 1m).
If the workflow is provisioned within the specified timeout, the API returns the created resources (same response as GetWorkflowStatus).
If the timeout is reached before provisioning completes, the API returns the workflow state without waiting further.

Updated the transport layer (ProvisionWorkflowTransportAction) to handle the timeout logic and ensure correct behavior during synchronous provisioning.

Success Response:

{
    "workflow_id": "K13IR5QBEpCfUu_-AQdU",
    "state": "COMPLETED",
    "resources_created": [
        {
            "workflow_step_name": "create_connector",
            "workflow_step_id": "create_connector_1",
            "resource_id": "LF3IR5QBEpCfUu_-Awd_",
            "resource_type": "connector_id"
        },
        {
            "workflow_step_id": "register_model_2",
            "workflow_step_name": "register_remote_model",
            "resource_id": "L13IR5QBEpCfUu_-BQdI",
            "resource_type": "model_id"
        },
        {
            "workflow_step_name": "deploy_model",
            "workflow_step_id": "deploy_model_3",
            "resource_id": "L13IR5QBEpCfUu_-BQdI",
            "resource_type": "model_id"
        }
    ]
}

TimeOut Response:

{
    "workflow_id": "SmACR5QBdrR0lYdqgHa9",
    "state": "PROVISIONING",
    "resources_created": [
        {
            "workflow_step_name": "create_connector",
            "workflow_step_id": "create_connector_1",
            "resource_type": "connector_id",
            "resource_id": "S2ACR5QBdrR0lYdqgXYK"
        },
        {
            "workflow_step_name": "register_remote_model",
            "workflow_step_id": "register_model_2",
            "resource_type": "model_id",
            "resource_id": "TWACR5QBdrR0lYdqgXZ-"
        }
    ]
}

Areas of Concern:

I have a few parts of the implementation that I believe can be further improved, particularly in ProvisionWorkflowTransportAction. Some of the logic feels a bit verbose and might not be the most efficient way to handle the timeout and synchronous execution. I’d appreciate the feedback from reviewers.

Related Issues

Resolves #967

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

dbwiddis

Generally looks good.

You need to handle -1 time value; my recommendation is you use that for the default "async" rather than null
You need to do stream version checks for the new (optional) workflow state in the response, and the new timeout parameter in the workflow request (unless you want to just keep it in the params map).

src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java

src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java

src/main/java/org/opensearch/flowframework/transport/WorkflowResponse.java

src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java

src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java

codecov · 2025-01-13T18:43:37Z

Codecov Report

Attention: Patch coverage is 45.34884% with 94 lines in your changes missing coverage. Please review.

Project coverage is 76.41%. Comparing base (5480cb4) to head (58b4f81).
Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
.../transport/ReprovisionWorkflowTransportAction.java	6.06%	29 Missing and 2 partials ⚠️
...rk/transport/ProvisionWorkflowTransportAction.java	9.09%	28 Missing and 2 partials ⚠️
...rch/flowframework/util/WorkflowTimeoutUtility.java	51.16%	19 Missing and 2 partials ⚠️
...ework/transport/CreateWorkflowTransportAction.java	76.47%	3 Missing and 1 partial ⚠️
...earch/flowframework/transport/WorkflowRequest.java	78.57%	0 Missing and 3 partials ⚠️
...ramework/transport/ReprovisionWorkflowRequest.java	71.42%	0 Missing and 2 partials ⚠️
...arch/flowframework/transport/WorkflowResponse.java	88.23%	0 Missing and 2 partials ⚠️
...h/flowframework/rest/RestCreateWorkflowAction.java	83.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #990      +/-   ##
============================================
- Coverage     77.57%   76.41%   -1.17%     
- Complexity      993     1010      +17     
============================================
  Files            99      100       +1     
  Lines          4714     4871     +157     
  Branches        431      453      +22     
============================================
+ Hits           3657     3722      +65     
- Misses          877      954      +77     
- Partials        180      195      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java

Signed-off-by: Junwei Dai <[email protected]>

Signed-off-by: Junwei Dai <[email protected]> # Conflicts: # src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java

Signed-off-by: Junwei Dai <[email protected]>

Signed-off-by: Junwei Dai <[email protected]> # Conflicts: # src/test/java/org/opensearch/flowframework/workflow/DeleteConnectorStepTests.java

Signed-off-by: Junwei Dai <[email protected]>

dbwiddis

LGTM with a few suggestions.

src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java

src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java

src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java

…, update error message Signed-off-by: Junwei Dai <[email protected]>

Signed-off-by: Junwei Dai <[email protected]>

joshpalis

Overall looks good to me, great work implementing this feature @junweid62 . A few comments

src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java

src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java

src/main/java/org/opensearch/flowframework/transport/WorkflowResponse.java

Signed-off-by: Junwei Dai <[email protected]>

opensearch-trigger-bot · 2025-01-16T23:36:29Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/flow-framework/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/flow-framework/backport-2.x
# Create a new branch
git switch --create backport/backport-990-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 33579a35a9d7a72cc0a2d44561b98bd64a79dd04
# Push it to GitHub
git push --set-upstream origin backport/backport-990-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/flow-framework/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-990-to-2.x.

dbwiddis · 2025-01-16T23:39:01Z

@junweid62 can you please manually backport this?

…-project#990) * Add synchronous execution option to workflow provisioning Signed-off-by: Junwei Dai <[email protected]> * code refactor Signed-off-by: Junwei Dai <[email protected]> * add change log Signed-off-by: Junwei Dai <[email protected]> * refactor code based on comment Signed-off-by: Junwei Dai <[email protected]> * fix spotless check Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to non-negative Signed-off-by: Junwei Dai <[email protected]> * Add synchronous execution to reprovision Signed-off-by: Junwei Dai <[email protected]> * remove unsued common value Signed-off-by: Junwei Dai <[email protected]> * add reprovision sync execution Signed-off-by: Junwei Dai <[email protected]> * fix test for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <[email protected]> * fix test name for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <[email protected]> * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message Signed-off-by: Junwei Dai <[email protected]> * fix spotless check Signed-off-by: Junwei Dai <[email protected]> * addressed some comments Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> (cherry picked from commit 33579a3)

…-project#990) * Add synchronous execution option to workflow provisioning Signed-off-by: Junwei Dai <[email protected]> * code refactor Signed-off-by: Junwei Dai <[email protected]> * add change log Signed-off-by: Junwei Dai <[email protected]> * refactor code based on comment Signed-off-by: Junwei Dai <[email protected]> * fix spotless check Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to non-negative Signed-off-by: Junwei Dai <[email protected]> * Add synchronous execution to reprovision Signed-off-by: Junwei Dai <[email protected]> * remove unsued common value Signed-off-by: Junwei Dai <[email protected]> * add reprovision sync execution Signed-off-by: Junwei Dai <[email protected]> * fix test for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <[email protected]> * fix test name for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <[email protected]> * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message Signed-off-by: Junwei Dai <[email protected]> * fix spotless check Signed-off-by: Junwei Dai <[email protected]> * addressed some comments Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]>

…ing(#990) (#1009) Add synchronous execution option to workflow provisioning (#990) * Add synchronous execution option to workflow provisioning * code refactor * add change log * refactor code based on comment * fix spotless check * Limit workflow timeout to a range of 1 to 300 seconds * Limit workflow timeout to a range of 1 to 300 seconds * Limit workflow timeout to non-negative * Add synchronous execution to reprovision * remove unsued common value * add reprovision sync execution * fix test for WorkflowTimeoutUtilityTests * fix test name for WorkflowTimeoutUtilityTests * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message * fix spotless check * addressed some comments --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]>

…-project#990) * Add synchronous execution option to workflow provisioning Signed-off-by: Junwei Dai <[email protected]> * code refactor Signed-off-by: Junwei Dai <[email protected]> * add change log Signed-off-by: Junwei Dai <[email protected]> * refactor code based on comment Signed-off-by: Junwei Dai <[email protected]> * fix spotless check Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <[email protected]> * Limit workflow timeout to non-negative Signed-off-by: Junwei Dai <[email protected]> * Add synchronous execution to reprovision Signed-off-by: Junwei Dai <[email protected]> * remove unsued common value Signed-off-by: Junwei Dai <[email protected]> * add reprovision sync execution Signed-off-by: Junwei Dai <[email protected]> * fix test for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <[email protected]> * fix test name for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <[email protected]> * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message Signed-off-by: Junwei Dai <[email protected]> * fix spotless check Signed-off-by: Junwei Dai <[email protected]> * addressed some comments Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> Signed-off-by: Junwei Dai <[email protected]>

junweid62 requested review from dbwiddis, owaiskazi19, joshpalis, ohltyler, amitgalitz and jackiehanyang as code owners January 8, 2025 21:30

github-actions bot added the backport 2.x backport PRs to 2.x branch label Jan 8, 2025

dbwiddis reviewed Jan 9, 2025

View reviewed changes

src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java Outdated Show resolved Hide resolved

dbwiddis reviewed Jan 13, 2025

View reviewed changes

src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java Outdated Show resolved Hide resolved

Junwei Dai added 13 commits January 15, 2025 10:53

Add synchronous execution option to workflow provisioning

98387f3

Signed-off-by: Junwei Dai <[email protected]>

code refactor

f3898cf

Signed-off-by: Junwei Dai <[email protected]>

add change log

80c5340

Signed-off-by: Junwei Dai <[email protected]>

refactor code based on comment

a2321e8

Signed-off-by: Junwei Dai <[email protected]>

fix spotless check

e690bd3

Signed-off-by: Junwei Dai <[email protected]>

Limit workflow timeout to a range of 1 to 300 seconds

014e6c1

Signed-off-by: Junwei Dai <[email protected]>

Limit workflow timeout to a range of 1 to 300 seconds

ce70128

Signed-off-by: Junwei Dai <[email protected]> # Conflicts: # src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java

Limit workflow timeout to non-negative

b9060b4

Signed-off-by: Junwei Dai <[email protected]>

Add synchronous execution to reprovision

edaafe8

Signed-off-by: Junwei Dai <[email protected]>

remove unsued common value

2584329

Signed-off-by: Junwei Dai <[email protected]>

add reprovision sync execution

15e052b

Signed-off-by: Junwei Dai <[email protected]> # Conflicts: # src/test/java/org/opensearch/flowframework/workflow/DeleteConnectorStepTests.java

fix test for WorkflowTimeoutUtilityTests

95816c9

Signed-off-by: Junwei Dai <[email protected]>

fix test name for WorkflowTimeoutUtilityTests

18a1dbb

Signed-off-by: Junwei Dai <[email protected]>

junweid62 force-pushed the provision-syncronosly branch from d6c0c53 to 18a1dbb Compare January 15, 2025 19:04

dbwiddis approved these changes Jan 15, 2025

View reviewed changes

Junwei Dai added 2 commits January 15, 2025 12:16

Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility…

6e8f147

…, update error message Signed-off-by: Junwei Dai <[email protected]>

fix spotless check

257564a

Signed-off-by: Junwei Dai <[email protected]>

joshpalis reviewed Jan 15, 2025

View reviewed changes

addressed some comments

eac9355

Signed-off-by: Junwei Dai <[email protected]>

Merge branch 'main' into provision-syncronosly

58b4f81

joshpalis approved these changes Jan 16, 2025

View reviewed changes

dbwiddis merged commit 33579a3 into opensearch-project:main Jan 16, 2025
19 of 20 checks passed

opensearch-trigger-bot bot added the backport-failed Applied to PRs when the automatic backport fails label Jan 16, 2025

junweid62 mentioned this pull request Jan 17, 2025

[backport 2.x] Add synchronous execution option to workflow provisioning(#990) #1009

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add synchronous execution option to workflow provisioning #990

Add synchronous execution option to workflow provisioning #990

junweid62 commented Jan 8, 2025 •

edited

Loading

dbwiddis left a comment

codecov bot commented Jan 13, 2025 •

edited

Loading

dbwiddis left a comment

joshpalis left a comment

opensearch-trigger-bot bot commented Jan 16, 2025

dbwiddis commented Jan 16, 2025

Add synchronous execution option to workflow provisioning #990

Add synchronous execution option to workflow provisioning #990

Conversation

junweid62 commented Jan 8, 2025 • edited Loading

Description

What’s Changed:

Areas of Concern:

Related Issues

Check List

dbwiddis left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 13, 2025 • edited Loading

Codecov Report

dbwiddis left a comment

Choose a reason for hiding this comment

joshpalis left a comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Jan 16, 2025

dbwiddis commented Jan 16, 2025

junweid62 commented Jan 8, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading